Repository navigation
Conversation
Replace DatabaseArtifactRepository::load/load_sized with load_meta (metadata columns only) and read_chunks (4 MiB substr slices of payload then delta_payload, guarded by revision + checkpoint hash, hashes verified while streaming) into an ArtifactSink. SQL and DuckDB hydration stream into staged cache files (published only after the read verified); export and the Forbid-growth size check read metadata only. Drops the bulk-read scope. Adds docs/runbooks/tc-822-artifact-payload-storage.md (Postgres STORAGE EXTERNAL operator step).
The slice statement was not on the read-only call allowlist, so a lost connection mid-slice was not retried (blackhole_mid_hydration CI failure).
… at three detections
…ice per part (TC-822)
…491/1a-local-durability
- CacheFileSink::commit removes every staged temp when a rename fails - sweep stale <file>.tmp.<pid>.<n> staging files before a SQL/DuckDB hydration (live in-process staging files are registered and never swept) - tc1274 lock-wait / hydration-bound tests no longer time a cold hydration - drop the dead SqlSizes seeded flag - read_chunks doc: SQLite memory bound is one blob, CPU O(n^2/chunk) - slice-size gate also checks the part's own size; read_artifact resets the sink on Ok(None); MemorySink reserves the part size; exhausted ArtifactChanged maps to SqlError::Unavailable (DuckDbError has no such variant, stays Internal)
A batch, or an execute with schema statements, ran each statement in SQLite autocommit, so a later failure left a committed prefix in the local file that the durable artifact (saved only after a fully successful request) did not have. Run such requests in an outermost SAVEPOINT and ROLLBACK on any error. Requests with a PRAGMA and single statements are unchanged; the Forbid guard keeps its own savepoint. Tests, docs/request-telemetry.md and CHANGELOG.
…1, slice 1a) synchronous=FULL on file-backed SQL databases (setting, env override), directory fsync for new database files, new directories and hydration renames, and a pre-COMMIT gate: per-database size cap and owner budget through SQLite max_page_count, disk headroom through a node-wide reservation bounded by the page cache. Over-budget writes roll back whole before COMMIT. 507 for the two new refusals. MemorySink size_hint is capped. Tests incl. kill -9 at each commit boundary. See docs/sql-local-durability.md.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
…libc dependency (TC-1491)
…ests # Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
…(TC-1491) The two limits are Sam's decision (card sql-storage-report-read): 0 = off, set by config. synchronous stays full. The mechanism is unchanged and tested with explicit values; owner-budget enforcement stays. Docs, config, changelog.
added 7 commits
October 20, 2026 06:49
Main's actor calls handle_message_authorized, so the request transaction wraps there (handle_request_authorized is main's former body). Fixtures that create tables hold admin; the PRAGMA test uses user_version (allow-listed). Adds a test that a WITH ... INSERT in a failed batch leaves nothing.
…491/1a-local-durability (TC-1491) The wrap, gate and reservation decision (needs_transaction) uses classify_sql (TC-1585 nested writes) and the request authority; the budget path has an authority entry point (execute_with_budget_authorized) that the route calls. Adds tests: WITH ... INSERT over budget/cap/disk is refused before COMMIT with no effects (core), and a route test over the remaining budget.
…ed; apply on Sam's answer)
Details: TC-1491
Details: TC-1636
Contributor
Author
|
CHANGES REQUESTED: see TC-1491 (head 053fc5d) |
added 3 commits
October 21, 2026 05:04
A request that writes (execute, batch, a named statement, or a query that writes or carries a PRAGMA) runs in one SAVEPOINT transaction: a failing statement or a deferred constraint at the commit rolls the whole request back, including a single statement that stops with INSERT OR FAIL or a column constraint with ON CONFLICT FAIL. One decision function, needs_transaction, says which requests are wrapped. The actor retires a connection that is still inside a transaction after its request, as it does when the TC-1586 reset fails. Details: TC-1491
# Conflicts: # CHANGELOG.md # tinycloud-core/src/sql/database.rs # tinycloud-core/tests/embedded_request_telemetry.rs
A full disk is reported as sql_disk_headroom whatever min_free_bytes is. A request that wrote nothing is neither gated nor reserved, and the progress check lets reads through; whether it wrote is asked of SQLite (sqlite3_txn_state). The growth of the main file that the next checkpoint will cause is held in the disk reservation from the commit until a checkpoint runs or the connection closes. A refused free-space read is the same error everywhere, and the guard that keeps a database from growing ends a refused request with ROLLBACK. Tests cover each of these. Details: TC-1491
… hold as implemented Details: TC-1491
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TC-1491 slice 1a, part B: make the node's local SQL commit durable, and refuse before COMMIT a write that would overshoot. Postgres stays master. Zero migrations, no schema change, no store/manifest types, no uploader. Design:
docs/sql-local-durability.md(in this PR).Merge order and what to review
This branch contains three other PRs by merge commit. Merge order: #376, then #383, then #384 (part A), then this.
5ccc305, bounded reads): approved, not merged.origin/tc1491/p-followups,83e3beb): merged in here by a merge commit, no force-push, its head unchanged. This PR touches TC-822/TC-1491: follow-ups to #376 (bounded reads) #383's code (MemorySink::size_hint,CacheFileSink::commit) and must merge after TC-822/TC-1491: follow-ups to #376 (bounded reads) #383.feb8ec9): merged in here by a merge commit. This PR does not carry A's logic twice; it extends A'shandle_message/is_atomic_requestintohandle_message_ctx/needs_transaction(below).Review only
c7cec06,0cee4f4,269986eandbf3ef73(the last turns the two limits off by default). The merge commits carry main (1.21.4), #376, #383 and #384 (A, now9df7eec). Everything else is already under review in the PRs above.Limits are Sam's decision (card
sql-storage-report-read); off by defaultmax_database_bytesandmin_free_bytesdefault to 0 = off (no cap; no headroom check, no reservation, no free-space reads), because an unmeasured cap could refuse real users' databases. They are set by config once Sam answers. The mechanism (cap throughmax_page_count, the page-cache WAL bound,DiskBudget) is unchanged and tested with explicit non-default values;the_limits_are_off_by_default_but_the_same_policy_enforces_when_setpins both sides.synchronous = "full"stays the default: it is the durability fix, not a limit.What stays on by default and can refuse something today's code allows: the owner-budget limit.
max_page_countis set from the owner's remaining quota (limit - used, from thestorage_checkread the route already makes). Today a write that overshoots a non-zero remainder is allowed once and the next is refused with 402; now the overshooting write itself is refused with 402 before COMMIT. The compared quantity is the database's page growth against the remainder (the unit the quota charges), so it is not stricter than the charge. Also on by default: every write request runs in a transaction (A's atomicity extended to single statements, so there is something to roll back), which is telemetry-visible (+2 native calls) but refuses nothing.What changes (invariants first)
For a file-backed SQL database, a write request now:
SAVEPOINT);max_page_count=min(per-database cap if set, pages now + owner budget / page size), so cap and owner budget are enforced exactly by SQLite, mid-statement;precommit_gate,sql/database.rs) that re-checks the page growth against the cap and budget and, ifmin_free_bytes > 0, reserves disk;COMMITs withPRAGMA synchronous = FULL, which fsyncs the WAL before COMMIT returns;Invariants the tests pin:
ROLLBACK, not a committed empty transaction): no effect on the file (checkpoint and export bytes identical), none on Postgres (zero repository calls), none in any later snapshot.FULL, its fsync). Success is also not returned before the Postgres save, unchanged.DiskBudget; a write's reservation shrinks to its checkpoint growth at COMMIT; the in-flight part is released when the request's outcome is known, and the checkpoint growth stays held until a checkpoint runs or the connection closes.Disk bound, and why it is sound for index amplification
At COMMIT SQLite appends one WAL frame (
page size + 24) per dirty page that has not already spilled to the WAL. Dirty pages are a subset of the connection's page cache, soSQLITE_DBSTATUS_CACHE_USEDbounds them whatever the SQL payload was. Index entries, every touched b-tree page, freelist and overflow pages are all just pages. A later checkpoint grows the main file by the transaction's page growth, which is reserved too. Reservation =ceil(cache_used / page size) * (page size + 24) + page growth * page size; admitted only iffree - other reservations - this >= min_free_bytes.Deviation from the design note (it said "WAL growth so far plus the unspilled bound"): WAL bytes that already spilled are on disk and already out of the free space the gate reads, so counting them again would double count. The gate reserves only what is still to be written.
The SQL parser admits no
CREATE TRIGGER, so indexes (and foreign keys) are the amplification a caller can cause; the amplification test uses six indexes.What is not exact or not guaranteed
free bytesis astatvfssnapshot: another process on the same filesystem can take the space after the check.min_free_bytesis the margin; the reservation covers this process only.memory_threshold, 10 MiB) have no local file. They get the size cap and owner budget but no fsync and no disk gate. Their durability is the Postgres save, as before. Store-master (1b) needs them file-backed.PRAGMA(admin only) is neither wrapped nor gated.Publicationcommands are not gated. DuckDB is untouched.FULLis necessary, not sufficient: it does not prove the CVM volume honours flushes (not tested here).SQLITE_FULL, now a rolled-back507 sql_disk_headroom).Settings (
[global.storage.sql])synchronous = "full"|"normal"(defaultfull;normalis the rollback flag),max_database_bytes = 0(no cap),min_free_bytes = 0(check off). Env:TINYCLOUD_STORAGE__SQL__SYNCHRONOUSetc. (the repo's canonical double-underscore form; the single-underscore legacy form works forSYNCHRONOUS, but would splitMIN_FREE_BYTESinto nested keys, so it is not documented for those). Your brief wrote[sql]; the existing path isglobal.storage.sql. Documented intinycloud.toml,CLAUDE.mdand the CHANGELOG.AGENTS.mddoes not exist in this checkout;CLAUDE.mdcarries the config table.fsync policy
synchronousis set inopen_connectionand, before the backup copy, inpromote_to_file(it was not set before; promotion also set WAL after the copy).-walfile (unix VFSUNIXFILE_DIRSYNCon a new WAL), not for the main file. Added:create_dir_all_durablefor the directories created for a database;sync_dirafterpromote_to_filecreates the file;sync_dirof the parent after the renames inCacheFileSink::commit(parts were alreadysync_all'd before rename).directories_created_for_a_database_are_made_durableonly proves the helper runs and is idempotent. The directory fsyncs are by inspection.Outcomes in this phase
Rejected (rolled back before COMMIT; includes the new refusals and a COMMIT that returns an error), committed (local COMMIT, and for file-backed with
FULLits fsync, and the Postgres save), unknown (unchanged TC-1274outcome_unknown). A failed Postgres save still discards local state; a never-released local commit is dropped at the next re-hydration. Details in the doc.COMMIT error (store-master note). If COMMIT's fsync fails after the WAL frames were written, the commit may later appear on recovery even though we reported the request as rejected. That is harmless while Postgres is master: the service discards the local state (new
SqlError::CommitFailedtriggersdiscard_local_state) and re-hydrates from Postgres, which never saw the write. Under store-master (1b+) it is an outcome-unknown case (keep the local WAL, answeroutcome_unknown). The code comment stays at the site (commit_error).Status codes: 507, not 413 (decision from evidence)
DatabaseSizeLimitandDiskHeadroomare 507 with distinct body prefixes (sql_database_size_limit:,sql_disk_headroom:). 413 would be misread:SQLService.mapHttpStatusToErrorCode:413 -> SQL_RESPONSE_TOO_LARGE(sdk-services/src/sql/SQLService.ts:553); a database cap is not a response size.KVServicetreats 413 asSTORAGE_LIMIT_REACHED, andTinyCloudQuotafiresonUpgradeRequiredon "402/413": a 413 here would prompt an upgrade for a cap that has nothing to do with the plan.routes/mod.rs).The SQL client maps an unknown status such as 507 to
NETWORK_ERROR(default branch). A small SDK follow-up could map the body prefixes; not done here. Owner budget keeps 402 (existingStorageWouldGrowmapping, TC-619 body), now also for a write that overshoots a non-zero remainder.Latency (indicative only, not a gate)
1000 small committed
INSERTs, one raw rusqlite connection, WAL, ext4 on LVM (/dev/mapper/vg0-root) on this host at load ~10; run twice (synchronous_latency_indicative,--ignored):So FULL costs about 0.2 ms per commit here. It does not include the actor, the transaction wrap or the Postgres save (which dominates a write). One 327 ms outlier in the first FULL round. Another disk (the CVM's) will differ.
Test evidence
New
tinycloud-core/tests/sql_local_durability.rs(12 tests, 1 ignored latency), plus unit tests:synchronous_*:PRAGMA synchronousreads 2 (FULL) / 1 (NORMAL) on opened and promoted connections and through an actor after promotion and reopen; journal mode is WAL. Config default and env override:sql_durability_settings_default_and_take_env_overrides(main.rs).an_amplified_transaction_is_rejected_before_commit_and_leaves_no_effects: 20 000 rows into 7 b-trees from a ~330-byte statement (several MiB of pages) with 1 MiB "free":DiskHeadroom, rows unchanged, checkpoint and export bytes identical; the same statement succeeds with room.the_unspilled_wal_bound_covers_what_commit_appends: for 1 to 100 000 rows plus an update of half of them, what COMMIT appended to the WAL is at most the bound computed just before it (and the cache did spill).a_batch_is_all_or_nothing_under_each_kind_of_rejection: cap, disk headroom, owner budget, statement error; none of the batch applies, checkpoint bytes identical.the_size_cap_and_owner_budget_are_enforced_by_sqlite_and_shrinking_writes_pass,a_database_over_the_cap_still_runs_writes_that_do_not_grow_it.disk_headroom_exhaustion_*: the floor alone refuses; recovers with no residue. Fake free-space provider (FreeSpacetrait); I did not mount a tmpfs (needs root).concurrent_databases_share_one_reservation_budget: database A's committed write holds ~400 KB; B's small write is refused with 300 KB free, passes after A's outcome releases.kill_minus_nine_at_each_commit_boundary_*: the test re-executes itself as a child, which dies by SIGKILL at the 6th commit atbefore_commit,after_commitandbefore_response, with every third request a rejected batch whose first statement ran. After reopen: every acknowledged commit is present, no rejected request or batch prefix is, and the one in-flight request is absent (before COMMIT) or present-but-unacknowledged (after). The child is asserted to die by SIGKILL.tc1491_reservation_adds_no_repository_round_trips(service, counting repository): the repository call sequence of two writes is identical with and without a budget, one persist per write; a budget-refused write makes zero repository calls and reserves nothing. At the route the only change is that thestorage_checkread that already ran per SQL write is reused (storageinstead offull_space); it adds no await.tc1491_memory_sink_survives_an_absurd_size_hint(for TC-822/TC-1491: follow-ups to #376 (bounded reads) #383's code): hints ofu64::MAX,usize::MAX,isize::MAX+1,1<<62,1<<40neither abort nor panic, output is exact, capacity stays under 64 MiB.tc1491_reservation_refusals_are_507_with_distinct_codes(route status and body prefixes).Tests that exercise the cap or headroom pass explicit values (
policy_with(&free, 1);0would be off). Existing tests changed:embedded_request_telemetry.rs(a write request now counts theSAVEPOINT/ROLLBACK: failing single statement 1 to 3), see below.Run:
cargo fmt --checkandcargo clippy --workspace --all-targets -D warningsclean;cargo test --workspace --exclude tinycloud-sdk-wasm --no-fail-fast: 0 failures on the final run. On a loaded host (load 15 to 37) three timing-sensitive tests (tc1274_a_*hydration*,legacy_freeze_sqlite_*) failed intermittently; each passes on rerun and in isolation, and they failed the same way on part A's branch before this change.Behaviour changes to know about
docs/request-telemetry.mdupdated.Operational notes
synchronous = "normal"restores the old fsync behaviour; the cap and headroom are off unless configured. The transaction wrap and the directory fsyncs are not switchable (revert the commit).Update: main (#389: TC-1581/1585/1586) merged in, head 8068e8d
Merged A's new head (
9df7eec, which already containsorigin/mainat1f43039) into this branch; one merge commit. Main's authority, classifier, authorizer scoping, DDL policy and PRAGMA allow-list are kept as they are.Classifier and the gate. One function decides the wrap, the gate and the reservation:
needs_transaction(request, caveats, authority). AQueryis wrapped whenparser::classify_sql(sql)says it is not read-only (it sees writes nested in a query tree, soWITH … INSERT/UPDATEare writes); a read-tier-only grant skips the parse (its writes are refused anyway).Execute,BatchandExecuteStatementare wrapped whatever they hold; a PRAGMA request is not wrapped, gated or reserved. A statement that does not parse is left to the request to report. The route'ssql_request_is_writewas already onclassify_sql(main), so a CTE write gets the owner budget read,max_page_count, the size cap and the pre-commit gate.Which entry point the route uses. Verified: the route calls
execute_without_growth_authorized(full space),execute_with_budget(budget) andexecute_authorized. Main's*_authorizedentry points did not cover the budget path:execute_with_budgettook the legacyStringability, which would have judged DDL with one preferred ability. NewSqlService::execute_with_budget_authorizedcarriesGrowthPolicy::Budgetwith the full authority; the legacyexecute_with_budgetdelegates to it; the route now calls the authorized one. All three*_authorizedpaths reach the samehandle_message_ctxin the actor.TC-1586 reset. It runs in the actor loop after the request returns, so after COMMIT/ROLLBACK and after the reservation was settled; a PRAGMA request never opened a transaction, so the reset's PRAGMAs never run inside one.
TC-1594 (retire order) and TC-1595 (user_version/application_id not persisted): not fixed here. Interaction with the retire order: none. The retire happens only after a request that held a non-report PRAGMA, which is never wrapped, gated or reserved. The reservation is released by the drop guard in
SqlService::execute_lockedwhen the request returns, not by the actor, on the same path as before; moving the connection close ahead of the deregister (TC-1594) does not change it. TC-1595 does not touch the gate either; it is a persist-after-PRAGMA question. Worth knowing for the owner: a PRAGMA request does not run the durable save unless it reports write targets, which is the TC-1595 behaviour.New tests.
a_nested_write_is_bounded_before_commit_like_any_other_write(tinycloud-core/tests/sql_local_durability.rs): aWITH … INSERTas aQueryis refused before COMMIT by the owner budget (StorageWouldGrow), the size cap (DatabaseSizeLimit) and the disk gate (DiskHeadroom); afterwards no rows, checkpoint bytes identical, WAL length unchanged, reservation 0; a small one passes. Mutation check: with the classifier returning "not a write" the test fails (the CTE write is admitted).tc1585_cte_wrapped_write_over_the_remaining_budget_is_402_and_leaves_nothing(route): over/invokewith a limit above usage (budget path, not a full space) a 64 KiB CTE insert is 402storage_quota_exceededand changes nothing; a small one is 200.tc1491_nested_write_cte_in_a_failed_batch_leaves_nothing.Fixtures and tests that had to change.
sql/admin(DDL needsschema).an_actor_honours_the_synchronous_setting_after_promotion_and_reopenreadPRAGMA synchronousthrough the actor; TC-1586's allow-list now refuses that (synchronousis a connection setting). It is nowan_actor_runs_under_either_synchronous_setting_after_promotion_and_reopen(promotion, a write,journal_mode= wal, reopen, data present). The setting itself is still asserted onopen_connectionandpromote_to_fileinsynchronous_is_full_on_every_file_backed_connection_and_normal_on_request. What is no longer tested: that the actor passespolicy.synchronousintoopen_connection(it cannot be observed through a request any more).tc1491_pragma_requests_keep_running_statement_by_statement(from A) usesPRAGMA user_version = 7instead offoreign_keys = OFF(refused by the allow-list).Behaviour changes in this PR from the merge: none to the feature itself. Carried over unchanged: size-hint cap,
max_database_bytesandmin_free_bytesdefault 0 (off),synchronous = fulldefault.Run. fmt, clippy
-D warnings;cargo test --workspace --exclude tinycloud-sdk-wasm --no-fail-fastagainst a private PostgreSQL 14 (TINYCLOUD_TEST_POSTGRES_URL),postgres_round_trip_budgetincluded: all pass exceptdb::test::postgres_space_filter_survives_a_collation_that_ignores_the_sentinel, which needs PostgreSQL 16 collation rules (the CI job uses 16; unrelated to this change). The iptablesblackhole_*tests were not forced on (noTINYCLOUD_TEST_BLACKHOLE); duckdb-feature core lib tests and the four duckdb node test binaries pass.